Support Room version 12 as default for a homeserver being tested#885
Open
jason-famedly wants to merge 6 commits into
Open
Support Room version 12 as default for a homeserver being tested#885jason-famedly wants to merge 6 commits into
jason-famedly wants to merge 6 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Updates Complement’s test suite to remain valid when the homeserver under test defaults to Matrix room version 12, accounting for (a) privileged room creators (MSC4289) and (b) the v12+ restriction around excluding m.room.create from auth_events (MSC4291).
Changes:
- Adjusts power level event construction in multiple tests to omit the room creator from
userswhen the room version uses privileged creators. - Updates several federation tests to conditionally omit
m.room.createfromauth_eventsfor room versions which disallow it. - Refactors a regression test to use a non-creator user to avoid the creator’s non-JSON-representable power level.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/msc3902/federation_room_join_partial_state_test.go | Conditionally omits creator from PL users and drops m.room.create from auth_events for v12+ in partial-state join scenarios. |
| tests/federation_room_event_auth_test.go | Adjusts crafted auth_events lists to avoid including m.room.create on room versions that disallow it. |
| tests/csapi/power_levels_test.go | Updates CSAPI power level tests for privileged creators and v12 behavior; switches one test to use a non-creator sender. |
| tests/csapi/apidoc_room_members_test.go | Updates reinvite scenario PL content to omit creator from users for privileged-creators rooms. |
| tests/csapi/apidoc_room_alias_test.go | Updates alias permission tests’ PL content to omit creator from users for privileged-creators rooms. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+125
to
129
| // This key should be missing for room v12+ | ||
| if gomatrixserverlib.MustGetRoomVersion(defaultRoomVersion).PrivilegedCreators() { | ||
| match.JSONKeyMissing("users." + client.GjsonEscape(alice.UserID)) | ||
| return nil | ||
| } else { |
|
looks good overall 👍 Better than what i came up with. Including the merge conflict between our fixes so you can check that.
shane@coffeelake:/run/media/shane/shane4tb-ent/repos/complement$ git merge main
Auto-merging tests/csapi/apidoc_room_members_test.go
CONFLICT (content): Merge conflict in tests/csapi/apidoc_room_members_test.go
Auto-merging tests/csapi/power_levels_test.go
CONFLICT (content): Merge conflict in tests/csapi/power_levels_test.go
Auto-merging tests/msc3902/federation_room_join_partial_state_test.go
Automatic merge failed; fix conflicts and then commit the result.
shane@coffeelake:/run/media/shane/shane4tb-ent/repos/complement$ gs
HEAD detached at jason-little/jason/roomv12
You have unmerged paths.
(fix conflicts and run "git commit")
(use "git merge --abort" to abort the merge)
Changes to be committed:
modified: .github/workflows/ci.yaml
modified: .gitignore
modified: README.md
modified: runtime/hs.go
new file: runtime/hs_continuwuity.go
new file: runtime/hs_test.go
new file: runtime/hs_tuwunel.go
modified: tests/csapi/apidoc_room_receipts_test.go
new file: tests/csapi/room_messages_pagination_test.go
modified: tests/csapi/rooms_state_test.go
modified: tests/csapi/sync_archive_test.go
modified: tests/federation_event_auth_test.go
modified: tests/federation_room_get_missing_events_test.go
modified: tests/msc3902/federation_room_join_partial_state_test.go
new file: tests/msc4186/docs/4186-simplified-sliding-sync.md
new file: tests/msc4186/main_test.go
new file: tests/msc4186/sliding_sync_test.go
new file: tests/msc4499/main_test.go
new file: tests/msc4499/msc4499_key_uniqueness_test.go
new file: tests/msc4500/main_test.go
new file: tests/msc4500/msc4500_test.go
modified: tests/room_summary_test.go
modified: tests/room_timestamp_to_event_test.go
Unmerged paths:
(use "git add <file>..." to mark resolution)
both modified: tests/csapi/apidoc_room_members_test.go
both modified: tests/csapi/power_levels_test.go
shane@coffeelake:/run/media/shane/shane4tb-ent/repos/complement$ gd
diff --cc tests/csapi/apidoc_room_members_test.go
index d4ea40b,b724594..0000000
--- a/tests/csapi/apidoc_room_members_test.go
+++ b/tests/csapi/apidoc_room_members_test.go
@@@ -103,19 -114,10 +116,26 @@@ func TestRoomMembers(t *testing.T)
alice.SendEventSynced(t, roomID, b.Event{
Type: "m.room.power_levels",
StateKey: &stateKey,
++<<<<<<< HEAD
+ Content: func() map[string]interface{} {
+ content := map[string]interface{}{
+ "invite": 100,
+ "users": map[string]int64{
+ bob.UserID: 100,
+ alice.UserID: 100,
+ },
+ }
+ if gomatrixserverlib.MustGetRoomVersion(defaultRoomVersion).PrivilegedCreators() {
+ delete(content["users"].(map[string]int64), alice.UserID)
+ }
+ return content
+ }(),
++=======
+ Content: map[string]interface{}{
+ "invite": 100,
+ "users": usersMap,
+ },
++>>>>>>> main
})
alice.MustLeaveRoom(t, roomID)
diff --cc tests/csapi/power_levels_test.go
index bbd5e90,be96a53..0000000
--- a/tests/csapi/power_levels_test.go
+++ b/tests/csapi/power_levels_test.go
@@@ -24,53 -23,42 +25,89 @@@ func TestDemotingUsersViaUsersDefault(
defer deployment.Destroy(t)
alice := deployment.Register(t, "hs1", helpers.RegistrationOpts{})
+ bob := deployment.Register(t, "hs1", helpers.RegistrationOpts{})
+
+ defaultRoomVersion := alice.GetDefaultRoomVersion(t)
+ roomVersion := alice.GetDefaultRoomVersion(t)
+ roomVer, err := strconv.Atoi(string(roomVersion))
+ if err != nil {
+ t.Skipf("non-numeric room version %q, skipping V12-conditional test", roomVersion)
+ }
+
+ usersMap := map[string]interface{}{}
+ if roomVer < 12 {
+ usersMap[alice.UserID] = 100
+ }
+
roomID := alice.MustCreateRoom(t, map[string]interface{}{
"preset": "public_chat",
++<<<<<<< HEAD
+ "power_level_content_override": func() map[string]interface{} {
+ power_level_content := map[string]interface{}{
+ "users_default": 100, // the default is 0
+ "users": map[string]int64{
+ alice.UserID: 100,
+ bob.UserID: 100,
+ },
+ "events": map[string]int64{},
+ "notifications": map[string]int64{},
+ }
+ // Remove the room creator if this is a v12+ room
+ if gomatrixserverlib.MustGetRoomVersion(defaultRoomVersion).PrivilegedCreators() {
+ delete(power_level_content["users"].(map[string]int64), alice.UserID)
+ }
+ return power_level_content
+ }(),
+ })
+
+ bob.MustJoinRoom(t, roomID, []spec.ServerName{
+ deployment.GetFullyQualifiedHomeserverName(t, "hs1"),
+ })
+
+ bob.SendEventSynced(t, roomID, b.Event{
+ Type: spec.MRoomPowerLevels,
+ StateKey: b.Ptr(""),
+ Content: func() map[string]interface{} {
+ content := map[string]interface{}{
+ "users_default": 40, // we change the default to 40. We should be able to do this.
+ "users": map[string]int64{
+ alice.UserID: 100,
+ bob.UserID: 100,
+ },
+ "events": map[string]int64{},
+ "notifications": map[string]int64{},
+ }
+ // Remove the room creator if this is a v12+ room
+ if gomatrixserverlib.MustGetRoomVersion(defaultRoomVersion).PrivilegedCreators() {
+ delete(content["users"].(map[string]int64), alice.UserID)
+ }
+ return content
+ }(),
++=======
+ "power_level_content_override": map[string]interface{}{
+ "users_default": 100, // the default is 0
+ "users": usersMap,
+ "events": map[string]int64{},
+ "notifications": map[string]int64{},
+ },
+ })
+
+ updatedUsersMap := map[string]interface{}{}
+ if roomVer < 12 {
+ updatedUsersMap[alice.UserID] = 100
+ }
+
+ alice.SendEventSynced(t, roomID, b.Event{
+ Type: "m.room.power_levels",
+ StateKey: b.Ptr(""),
+ Content: map[string]interface{}{
+ "users_default": 40, // we change the default to 40. We should be able to do this.
+ "users": updatedUsersMap,
+ "events": map[string]int64{},
+ "notifications": map[string]int64{},
+ },
++>>>>>>> main
})
}
@@@ -122,18 -104,23 +159,35 @@@ func TestPowerLevels(t *testing.T)
}),
func(body gjson.Result) error {
++<<<<<<< HEAD
+ // This key should be missing for room v12+
+ if gomatrixserverlib.MustGetRoomVersion(defaultRoomVersion).PrivilegedCreators() {
+ match.JSONKeyMissing("users." + client.GjsonEscape(alice.UserID))
++=======
+ userDefault := int(body.Get("users_default").Num)
+ thisUserRes := body.Get("users." + client.GjsonEscape(alice.UserID))
+
+ roomVersion := alice.GetDefaultRoomVersion(t)
+ roomVer, err := strconv.Atoi(string(roomVersion))
+ if err != nil {
+ return nil // non-numeric version, skip assertion
+ }
+ if roomVer >= 12 && !thisUserRes.Exists() {
+ return nil // In V12+, creators are implicitly privileged and may be absent from 'users'
+ }
+
+ thisUser := int(thisUserRes.Num)
+ if thisUser > userDefault {
++>>>>>>> main
return nil
} else {
- return fmt.Errorf("expected room creator (%d) to have a higher-than-default powerlevel (which is %d)", thisUser, userDefault)
+ userDefault := int(body.Get("users_default").Num)
+ thisUser := int(body.Get("users." + client.GjsonEscape(alice.UserID)).Num)
+ if thisUser > userDefault {
+ return nil
+ } else {
+ return fmt.Errorf("expected room creator (%d) to have a higher-than-default powerlevel (which is %d)", thisUser, userDefault)
+ }
}
},
)
@@@ -144,19 -143,11 +210,16 @@@
// note: these need to be floats to allow a roundtrip comparison
PLContent := map[string]interface{}{
"invite": 100.0,
- "users": map[string]interface{}{
- alice.UserID: 100.0,
- "@random-other-user:their.home": 20.0,
- },
+ "users": usersMap,
}
+ // Rooms versioned 12+ do not allow having the room creator in the 'users' object, so just remove it
+ if gomatrixserverlib.MustGetRoomVersion(defaultRoomVersion).PrivilegedCreators() {
+ delete(PLContent["users"].(map[string]interface{}), alice.UserID)
+ }
+
eventId := alice.SendEventSynced(t, roomID, b.Event{
- Type: "m.room.power_levels",
+ Type: spec.MRoomPowerLevels,
StateKey: b.Ptr(""),
Content: PLContent,
})
@@@ -176,23 -167,23 +239,39 @@@
// sytest: PUT power_levels should not explode if the old power levels were empty
t.Run("PUT power_levels should not explode if the old power levels were empty", func(t *testing.T) {
+ roomVersion := alice.GetDefaultRoomVersion(t)
+ roomVer, err := strconv.Atoi(string(roomVersion))
+ if err != nil {
+ t.Skipf("non-numeric room version %q, skipping V12-conditional test", roomVersion)
+ }
+ usersMap := map[string]interface{}{}
+ if roomVer < 12 {
+ usersMap[alice.UserID] = 100
+ }
+
// Absence of an "events" key
alice.SendEventSynced(t, roomID, b.Event{
- Type: "m.room.power_levels",
+ Type: spec.MRoomPowerLevels,
StateKey: b.Ptr(""),
++<<<<<<< HEAD
+ Content: func() map[string]interface{} {
+ PLContent := map[string]interface{}{
+ "users": map[string]int64{
+ alice.UserID: 100,
+ },
+ }
+ // Rooms versioned 12+ do not allow having the room creator in the 'users' object, so just remove the
+ // single user so the empty `users` object is present
+ if gomatrixserverlib.MustGetRoomVersion(defaultRoomVersion).PrivilegedCreators() {
+ delete(PLContent["users"].(map[string]int64), alice.UserID)
+ }
+ return PLContent
+ }(),
++=======
+ Content: map[string]interface{}{
+ "users": usersMap,
+ },
++>>>>>>> main
})
// Absence of a "users" key
@@@ -202,46 -193,29 +281,73 @@@
Content: map[string]interface{}{},
})
++<<<<<<< HEAD
+ // This part of the test should check that sending a power_levels event fails. As then the previous
+ // power_levels event should not have changed. Depending on the room version, this can be done in one
+ // of two different ways:
+ if gomatrixserverlib.MustGetRoomVersion(defaultRoomVersion).PrivilegedCreators() {
+ // For rooms with privileged creators(MSC4289), try and send an event with the room creator in the
+ // `users` object, which should be prohibited with a 400 error code. An empty `users` object appears
+ // to be allowed, hence the difference from the below condition for other room versions.
+ res := alice.Do(
+ t,
+ "PUT",
+ []string{"_matrix", "client", "v3", "rooms", roomID, "state", spec.MRoomPowerLevels},
+ client.WithJSONBody(t, map[string]interface{}{
+ "users": map[string]int64{
+ alice.UserID: 100,
+ },
+ }),
+ )
+ must.MatchResponse(t, res, match.HTTPResponse{
+ StatusCode: 400,
+ })
+
+ } else {
+ // Prior to rooms using privileged creators, an empty `users` object would be prohibited and should
+ // give a 403 (not a 500)
+ res := alice.Do(
+ t,
+ "PUT",
+ []string{"_matrix", "client", "v3", "rooms", roomID, "state", spec.MRoomPowerLevels},
+ client.WithJSONBody(t, map[string]interface{}{
+ "users": map[string]int64{},
+ }),
+ )
+ must.MatchResponse(t, res, match.HTTPResponse{
+ StatusCode: 403,
+ })
+
+ }
+
+ // Test if the old state still exists
+ content := alice.MustGetStateEventContent(t, roomID, spec.MRoomPowerLevels, "")
+ must.MatchGJSON(t, content, match.JSONKeyMissing("users"))
++=======
+ // This should give a 403 (not a 500)
+ res := alice.Do(
+ t,
+ "PUT",
+ []string{"_matrix", "client", "v3", "rooms", roomID, "state", "m.room.power_levels"},
+ client.WithJSONBody(t, map[string]interface{}{
+ "users": map[string]string{},
+ }),
+ )
+ expectedStatus := 403
+ if roomVer >= 12 {
+ expectedStatus = 200
+ }
+ must.MatchResponse(t, res, match.HTTPResponse{
+ StatusCode: expectedStatus,
+ })
+
+ // Test if the state was updated (V12) or rejected (V11)
+ content := alice.MustGetStateEventContent(t, roomID, "m.room.power_levels", "")
+ if roomVer >= 12 {
+ must.MatchGJSON(t, content, match.JSONKeyEqual("users", map[string]interface{}{}))
+ } else {
+ must.MatchGJSON(t, content, match.JSONKeyMissing("users"))
+ }
++>>>>>>> main
})
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Largely, this is just "on-the-fly" adjustment of power levels to not contain the room creator, and removing the
m.room.createevent fromauth_eventsusing a splice. The former was easy to base on theRoomVersionhaving thePrivilegedCreatorsproperty(for MSC4289), the latter I chose to go with the existing propertyDomainlessRoomIDsas MSC4291 suggests it is where this requirement for nom.room.createevent inauth_eventscomes from.For the regression test
TestDemotingUsersViaUsersDefault, I opted to change the basis of the test to a different user, to work around the room's creator having an infinite power level.To fix
TestOutboundFederationIgnoresMissingEventWithBadJSONForRoomVersion6, thisDepends on:
Pull Request Checklist
Signed-off-by:
Jason Little <j.little@famedly.com>